haiku support #154

hoanga · 2023-09-29T09:16:40Z

hello!

this changeset fixes an error that occurs when trying to compile on haiku that would typically show up like so:

...
...
error[E0432]: unresolved import `rustix::pipe::pipe_with`
   --> src/poll.rs:450:30
    |
450 |     use rustix::pipe::{pipe, pipe_with, PipeFlags};
    |                              ^^^^^^^^^ no `pipe_with` in `pipe`

For more information about this error, try `rustc --explain E0432`.
warning: `polling` (lib) generated 1 warning
error: could not compile `polling` due to previous error; 1 warning emitted

haiku doesn't offer pipe2() which appears to be the problem. this changeset updates the haiku case to prefer pipe() instead. an example test run with patch applied is shown below:

> uname -a
Haiku shredder 1 hrev57294 Sep 24 2023 07:34:59 x86_64 x86_64 Haiku

> cargo build && cargo test
   Compiling libc v0.2.148
   Compiling rustix v0.38.14
   Compiling bitflags v2.4.0
   Compiling tracing-core v0.1.31
   Compiling pin-project-lite v0.2.13
   Compiling cfg-if v1.0.0
   Compiling tracing v0.1.37
   Compiling errno v0.3.3
   Compiling polling v3.1.0 (/boot/home/src/git/rust-libs/polling)
    Finished dev [unoptimized + debuginfo] target(s) in 12.35s
   Compiling easy-parallel v3.3.1
   Compiling fastrand v2.0.1
   Compiling polling v3.1.0 (/boot/home/src/git/rust-libs/polling)
    Finished test [unoptimized + debuginfo] target(s) in 19.93s
     Running unittests src/lib.rs (target/debug/deps/polling-0b8e69d1ee132cd7)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/concurrent_modification.rs (target/debug/deps/concurrent_modification-d64066843130bb94)

running 2 tests
test concurrent_add ... ok
test concurrent_modify ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.11s

     Running tests/io.rs (target/debug/deps/io-8d88ea176e7058d2)

running 2 tests
test insert_twice ... ok
test basic_io ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/many_connections.rs (target/debug/deps/many_connections-1718dd2743f7b3ba)

running 1 test
test many_connections ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

     Running tests/multiple_pollers.rs (target/debug/deps/multiple_pollers-00e7f36939f0061d)

running 3 tests
test edge_triggered ... ok
test level_triggered ... ok
test oneshot_triggered ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 6.70s

     Running tests/notify.rs (target/debug/deps/notify-0524de0c61bbacd2)

running 2 tests
test simple ... ok
test concurrent ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/other_modes.rs (target/debug/deps/other_modes-0a6db0bb1c2c5be2)

running 3 tests
test edge_triggered ... ok
test level_triggered ... ok
test edge_oneshot_triggered ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/precision.rs (target/debug/deps/precision-69006f184ddddf9e)

running 2 tests
test below_ms ... ok
test above_ms ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 9.84s

     Running tests/timeout.rs (target/debug/deps/timeout-2ebbe2c5ba4fc740)

running 2 tests
test non_blocking ... ok
test twice ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.83s

     Running tests/windows_post.rs (target/debug/deps/windows_post-ad5e81d62a192ef5)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/windows_waitable.rs (target/debug/deps/windows_waitable-898e88b8f9e88518)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests polling

running 17 tests
test src/lib.rs - Events::clear (line 792) - compile ... ok
test src/lib.rs - (line 20) - compile ... ok
test src/lib.rs - Events::is_empty (line 830) ... ok
test src/lib.rs - Events::capacity (line 845) ... ok
test src/lib.rs - Events::len (line 815) ... ok
test src/lib.rs - Events::iter (line 771) ... ok
test src/lib.rs - Poller::add (line 434) - compile ... ok
test src/lib.rs - Events::with_capacity (line 750) ... ok
test src/lib.rs - Poller::modify (line 505) - compile ... ok
test src/lib.rs - Events::new (line 729) ... ok
test src/lib.rs - Poller::modify (line 517) - compile ... ok
test src/lib.rs - Poller::modify (line 530) - compile ... ok
test src/lib.rs - Poller::modify (line 543) - compile ... ok
test src/lib.rs - Poller::delete (line 594) ... ok
test src/lib.rs - Poller::new (line 364) ... ok
test src/lib.rs - Poller::notify (line 678) ... ok
test src/lib.rs - Poller::wait (line 631) ... ok

test result: ok. 17 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.81s

* pipe2() not available on haiku

notgull

Could you add tests to CI to make sure it builds on Haiku? Just a cargo check would suffice.

src/poll.rs

notgull · 2023-09-30T01:55:03Z

Actually this could probably be folded into the pipe-based impl used by non-ESP-IDF systems. Just replace the pipe_with() call with Err(ENOSYS).

hoanga · 2023-09-30T06:20:33Z

i have added an extra ci check that runs cargo check for haiku (x86-64) per suggestion.

in regards to replacing pipe_with() call with Err(ENOSYS), i am not sure i completely follow what you're getting at. does that also mean replacing the use declaration of rustix::pipe::pipe_with as well?

notgull · 2023-09-30T15:03:01Z

in regards to replacing pipe_with() call with Err(ENOSYS), i am not sure i completely follow what you're getting at. does that also mean replacing the use declaration of rustix::pipe::pipe_with as well?

I would just modify the pipe-based notifier to do something like this:

#[cfg(not(target_os = "haiku"))]
let pipe = pipe_with(...).ok();

#[cfg(target_os = "haiku")]
let pipe = None;

let (read, write) = pipe.unwrap_or_else(|| {
    let (read, write) = pipe();
    ...
});

hoanga · 2023-09-30T23:56:43Z

ah, gotcha. makes sense now. i have pushed some updates that folds the haiku and non-ESP-IDF sections as per suggestion and also keeps tests passing (and on haiku as well). thanks for the pointers!

notgull · 2023-10-01T00:04:33Z

src/poll.rs

+            #[cfg(not(target_os = "haiku"))]
            let (read_pipe, write_pipe) = pipe_with(PipeFlags::CLOEXEC).or_else(|_| {
                let (read_pipe, write_pipe) = pipe()?;
                fcntl_setfd(&read_pipe, fcntl_getfd(&read_pipe)? | FdFlags::CLOEXEC)?;
                fcntl_setfd(&write_pipe, fcntl_getfd(&write_pipe)? | FdFlags::CLOEXEC)?;
                io::Result::Ok((read_pipe, write_pipe))
            })?;

+            #[cfg(target_os = "haiku")]
+            let pipe_with = None;
+
+            #[cfg(target_os = "haiku")]
+            let (read_pipe, write_pipe) = pipe_with.unwrap_or_else(|| {
+                let (read_pipe, write_pipe) = pipe()?;
+                fcntl_setfd(&read_pipe, fcntl_getfd(&read_pipe)? | FdFlags::CLOEXEC)?;
+                fcntl_setfd(&write_pipe, fcntl_getfd(&write_pipe)? | FdFlags::CLOEXEC)?;
+                io::Result::Ok((read_pipe, write_pipe))
+            })?;
+


I'd rather not duplicate the pipe creation code twice. Instead, can you make a closure that creates the pipe, then call it in the Haiku case and use it in or_else in the other case?

notgull · 2023-10-01T17:22:35Z

src/poll.rs

+            #[cfg(not(target_os = "haiku"))]
+            let pipe_init = pipe_with;
+
+            #[cfg(target_os = "haiku")]
+            let pipe_init = |_| {


Suggested change

#[cfg(not(target_os = "haiku"))]

let pipe_init = pipe_with;

#[cfg(target_os = "haiku")]

let pipe_init = |_| {

let fallback_pipe = || {

notgull · 2023-10-01T17:23:53Z

src/poll.rs

+            };
+
+            let (read_pipe, write_pipe) = pipe_init(PipeFlags::CLOEXEC)?;


Suggested change

};

let (read_pipe, write_pipe) = pipe_init(PipeFlags::CLOEXEC)?;

#[cfg(not(target_os = "haiku"))]

let (read_pipe, write_pipe) = pipe_with(PipeFlags::CLOEXEC).or_else(fallback_pipe)?;

#[cfg(target_os = "haiku")]

let (read_pipe, write_pipe) = fallback_pipe()?;

updated per suggestions

notgull

Thanks!

hoanga added 2 commits September 29, 2023 10:01

use pipe instead of pipe2 for haiku

c1919c4

* pipe2() not available on haiku

formatting update

537daac

notgull requested changes Sep 29, 2023

View reviewed changes

src/poll.rs Outdated Show resolved Hide resolved

hoanga added 2 commits September 30, 2023 07:02

fix typo

2897b72

add ci check for haiku

560abda

consolidate haiku and non-esp notifier

bcb4aa6

notgull requested changes Oct 1, 2023

View reviewed changes

hoanga added 3 commits October 1, 2023 00:44

address ci

c6423eb

reduce pipe creation duplicates

6993bf4

address ci

fb1b0cc

notgull requested changes Oct 1, 2023

View reviewed changes

notgull approved these changes Oct 2, 2023

View reviewed changes

notgull merged commit 99a32b7 into smol-rs:master Oct 2, 2023
24 checks passed

notgull mentioned this pull request Oct 2, 2023

v3.2.0 #156

Merged

make fallback_pipe handle more

b150d3d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

haiku support #154

haiku support #154

hoanga commented Sep 29, 2023

notgull left a comment

notgull commented Sep 30, 2023

hoanga commented Sep 30, 2023

notgull commented Sep 30, 2023

hoanga commented Sep 30, 2023

notgull Oct 1, 2023

hoanga Oct 1, 2023

notgull Oct 1, 2023

notgull Oct 1, 2023

hoanga Oct 2, 2023

notgull left a comment

		};

		let (read_pipe, write_pipe) = pipe_init(PipeFlags::CLOEXEC)?;

haiku support #154

haiku support #154

Conversation

hoanga commented Sep 29, 2023

notgull left a comment

Choose a reason for hiding this comment

notgull commented Sep 30, 2023

hoanga commented Sep 30, 2023

notgull commented Sep 30, 2023

hoanga commented Sep 30, 2023

notgull Oct 1, 2023

Choose a reason for hiding this comment

hoanga Oct 1, 2023

Choose a reason for hiding this comment

notgull Oct 1, 2023

Choose a reason for hiding this comment

notgull Oct 1, 2023

Choose a reason for hiding this comment

hoanga Oct 2, 2023

Choose a reason for hiding this comment

notgull left a comment

Choose a reason for hiding this comment